Skip to content

Conversation

denisJaved
Copy link

@denisJaved denisJaved commented Feb 21, 2025

Stores last user selected game version and platform (aka modloader) to retrive them before openning download modal.

Resolves #2332

PS: It's my first contribution to modrinth. I'm sorry if i did smth wrong

Stores selected game version and platform(modloader) in localStorage to retrive them when download modal openned
@denisJaved denisJaved changed the title Store last selected game version and platform for download modal feat(frontend): Store last selected game version and platform for download modal Feb 21, 2025
@denisJaved denisJaved force-pushed the remember-game-version-and-platform branch from 30a24fb to 9fdac5a Compare February 22, 2025 08:06
@IMB11 IMB11 added the enhancement New feature or request label May 29, 2025
@IMB11 IMB11 requested a review from Copilot May 29, 2025 13:22
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR enhances the download modal by persisting the last user-selected game version and platform using localStorage. Key changes include:

  • Replacing ButtonStyled blocks with Accordions for game version and platform selection.
  • Introducing helper functions (setSelectedGameVersion and setSelectedPlatform) to update both reactive refs and localStorage.
  • Loading persisted selections on the client when the page is loaded.
Comments suppressed due to low confidence (2)

apps/frontend/src/pages/[type]/[id].vue:328

  • Confirm that changing this condition from v-else-if to v-if does not inadvertently render the platform Accordion for resourcepack projects, potentially affecting the intended download flow.
v-if="project.project_type !== 'resourcepack'"

apps/frontend/src/pages/[type]/[id].vue:880

  • [nitpick] Ensure that using localStorage with the keys 'selected_game_version' and 'selected_platform' is consistent with the application standards, and consider any necessary type handling for stored values.
if (import.meta.client) {

Copy link
Member

@IMB11 IMB11 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall I understand why you made this PR, the code quality is not great and some things were removed that shouldn't have been removed. Please check my comments, thanks!

function setSelectedGameVersion(version) {
userSelectedGameVersion.value = version;
if (import.meta.client) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than actually calling localStorage, it might be best to use useLocalStorage composable provided by vueuse, which handles the import.meta.client stuff for you automatically.

Also, make sure that it correctly falls back to undefined/not set if the local storage game version isn't actually supported by the project.

</div>
</div>

<div class="mx-auto flex w-fit flex-col gap-2">
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why was this removed? "Error: no game versions found"

:label="`Show all versions`"
:disabled="!!versionFilter"
/>
</Accordion>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here, why was this removed?

IMB11

This comment was marked as off-topic.

@denisJaved denisJaved closed this Aug 4, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request frontend
Development

Successfully merging this pull request may close these issues.

The download modal should remember what was selected
3 participants